Skip to content

fix(humanize): tolerate moving targets after scroll - #355

Closed
Kumario1 wants to merge 2 commits into
CloakHQ:mainfrom
Kumario1:fix/human-stable-check-budget
Closed

fix(humanize): tolerate moving targets after scroll#355
Kumario1 wants to merge 2 commits into
CloakHQ:mainfrom
Kumario1:fix/human-stable-check-budget

Conversation

@Kumario1

@Kumario1 Kumario1 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #329.

Summary

  • Treat the extra post-scroll stable-position wait as best-effort for click, dblclick, and hover when the element remains in motion.
  • Keep pre-scroll actionability and final pointer-event checks strict, then use the latest bounding box for the target.
  • Add JS and Python regressions for moving targets after scroll.

Verification

  • cd js && npm test -- humanize.test.ts
  • cd js && npm run typecheck
  • python -m pytest tests/test_humanize_unit.py::TestPerCallTimeoutForwarding tests/test_humanize_unit.py::TestPostScrollStability tests/test_humanize_unit.py::TestPointerEventsFailOpen::test_handle_failopen_returns_on_evaluate_error tests/test_humanize_unit.py::TestPointerEventsFailOpen::test_locator_failopen_returns_on_evaluate_error tests/test_humanize_unit.py::TestPointerEventsFailOpen::test_handle_still_raises_when_covered -q

Note: full tests/test_humanize_unit.py is not clean in this local environment because pytest-asyncio is unavailable and several existing tests launch real browsers or hit external pages.

@Cloak-HQ

Copy link
Copy Markdown
Contributor

Thanks for digging into this, and for the clean, well scoped patch.

We reproduced #329 before reviewing, then ran your change against it. Sharing the results because they point somewhere unexpected.

The test page puts a button below the fold and grows content above it every 50ms, so the button drifts continuously at roughly 60px/s. Same page, four cases:

native Playwright click        PASS   0.1s
humanize click (main)          FAIL  15.2s   ElementNotStableError
humanize click (this PR)       FAIL  15.8s   TimeoutError: bounding_box: Timeout 1ms exceeded
humanize, motion stops at 3s   PASS   6.7s   (passes on main too)

Two findings.

First, the click still fails with the patch applied.
Swallowing ElementNotStableError lets ensure_stable consume the whole remaining budget.
Execution then continues to the next line, the bounding_box read, where max(1, _remaining_ms()) clamps to 1ms and times out immediately.
The user ends up with Timeout 1ms exceeded in place of a message that actually described the problem.

Second, and this is what changed our mind about the underlying issue.
We tried several ways to make the click land, including re-reading the box after the mouse travel and re-aiming at the element centre.
All of them missed, by 5px and then 12px.

The cause is structural.
Humanize moves the cursor along a Bézier path and adds a pre-click aim delay, so roughly half a second passes between reading the position and dispatching mousedown.
An element moving 60px/s travels further than its own height in that window.
Native Playwright succeeds only because it computes coordinates and fires the event in the same instant, with no travel.

So on a page that never settles, the element genuinely cannot be clicked at human speed, and the existing exception is reporting that correctly.
Relaxing the check does not produce a successful click, it produces a click on whatever moved into that position.

If the region does eventually settle, clicking it normally already works.
If it never settles, the supported escape hatch is a native click that skips the behavioural layer:

page._original.click("div.show-more")

We measured that at 0.1s on the never settling page.

Given all that, we are going to decline this one.
Not because the report is invalid, #329 is a real frustration, but because the check is what stands between the user and a silent misclick.

Where we think the real gap is: the error message says what failed but not what to do about it.
Pointing users at page._original directly from the exception text would have saved this whole round trip.
Happy to look at a patch along those lines.

@Cloak-HQ

Copy link
Copy Markdown
Contributor

Following up, because my earlier comment got the explanation wrong.

I said the stability check is what stands between the user and a silent misclick. It is not. main was already producing them.

After instrumenting the pipeline, the real cause sits two lines from where this PR was looking. We scroll the element into view, then wait for it to settle, and never scroll again. On a reflowing page the element gets pushed off screen during that wait, so we click at coordinates outside the viewport and hit nothing. check_pointer_events detects it, but near its deadline the bounding_box timeout clamps to ~1ms and throws, and a throw is treated as indeterminate and fails open. A proven miss became "unknown" and the click went through.

Measured on main: a page reflowing 10 to 25 seconds gives a missed click after ~32s with no exception.

So your instinct about this area was right. The stability check firing is a symptom of exactly the pages the real bug affects.

Fix is in #468. A review from you would be welcome.

Two retractions. page._original.click() was poor advice, it is a raw click with no cursor movement, which is the signal the behavioural layer exists to hide. And my claim that such an element simply cannot be clicked at human speed described the synthetic page I had built, not real pages.

The decline still holds on the measurements, this patch still fails and replaces a useful message with Timeout 1ms exceeded. But the reasoning I gave was wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

idx=2, err=Element 'div.parent-comment >> nth=1 >> div.reply-container >> div.show-more' failed stable check: element position is still changing

2 participants